From 1ef09734ed86eaa46df4c382b0fa8451648fdb03 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 23 Sep 2014 15:22:14 -0700 Subject: [PATCH] Refine dependencies on dev-deps Currently whenever a dev-dep is brought in to the build process the entire library is rebuilt, but this is just unnecessary recompilation because the library *can't* depend on the dev-dep. This commit refines the dependency graph so the lib stage only depends on transitive dependencies (non-dev-deps), and a new stage for tests was added which depends on the packages libraries *and* the dev-deps. This way only the test are rebuilt when dev-deps change, not libraries. --- src/cargo/ops/cargo_rustc/job_queue.rs | 45 +++++++++++++++----- src/cargo/ops/cargo_rustc/mod.rs | 28 ++++++++----- tests/test_cargo_compile_git_deps.rs | 6 +-- tests/test_cargo_compile_path_deps.rs | 57 +++++++++++++++++++++++++- 4 files changed, 110 insertions(+), 26 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/job_queue.rs b/src/cargo/ops/cargo_rustc/job_queue.rs index ad6aea045..2eaa8871a 100644 --- a/src/cargo/ops/cargo_rustc/job_queue.rs +++ b/src/cargo/ops/cargo_rustc/job_queue.rs @@ -1,7 +1,7 @@ use std::collections::hashmap::{HashMap, HashSet, Occupied, Vacant}; use term::color::YELLOW; -use core::{Package, PackageId, Resolve}; +use core::{Package, PackageId, Resolve, PackageSet}; use util::{Config, TaskPool, DependencyQueue, Fresh, Dirty, Freshness}; use util::{CargoResult, Dependency, profile}; @@ -19,6 +19,7 @@ pub struct JobQueue<'a, 'b> { tx: Sender, rx: Receiver, resolve: &'a Resolve, + packages: &'a PackageSet, active: uint, pending: HashMap<(&'a PackageId, TargetStage), PendingBuild>, state: HashMap<&'a PackageId, Freshness>, @@ -49,13 +50,14 @@ pub enum TargetStage { StageCustomBuild, StageLibraries, StageBinaries, - StageEnd, + StageTests, } type Message = (PackageId, TargetStage, Freshness, CargoResult<()>); impl<'a, 'b> JobQueue<'a, 'b> { - pub fn new(resolve: &'a Resolve, config: &mut Config) -> JobQueue<'a, 'b> { + pub fn new(resolve: &'a Resolve, packages: &'a PackageSet, + config: &mut Config) -> JobQueue<'a, 'b> { let (tx, rx) = channel(); JobQueue { pool: TaskPool::new(config.jobs()), @@ -63,6 +65,7 @@ impl<'a, 'b> JobQueue<'a, 'b> { tx: tx, rx: rx, resolve: resolve, + packages: packages, active: 0, pending: HashMap::new(), state: HashMap::new(), @@ -81,7 +84,7 @@ impl<'a, 'b> JobQueue<'a, 'b> { }; // Add the package to the dependency graph - self.queue.enqueue(&self.resolve, Fresh, + self.queue.enqueue(&(self.resolve, self.packages), Fresh, (pkg.get_package_id(), stage), (pkg, jobs)); } @@ -193,8 +196,10 @@ impl<'a, 'b> JobQueue<'a, 'b> { } } -impl<'a> Dependency<&'a Resolve> for (&'a PackageId, TargetStage) { - fn dependencies(&self, resolve: &&'a Resolve) +impl<'a> Dependency<(&'a Resolve, &'a PackageSet)> + for (&'a PackageId, TargetStage) +{ + fn dependencies(&self, &(resolve, packages): &(&'a Resolve, &'a PackageSet)) -> Vec<(&'a PackageId, TargetStage)> { // This implementation of `Dependency` is the driver for the structure // of the dependency graph of packages to be built. The "key" here is @@ -204,18 +209,38 @@ impl<'a> Dependency<&'a Resolve> for (&'a PackageId, TargetStage) { // the start state which depends on the ending state of all dependent // packages (as determined by the resolve context). let (id, stage) = *self; + let pkg = packages.iter().find(|p| p.get_package_id() == id).unwrap(); + let deps = resolve.deps(id).into_iter().flat_map(|a| a) + .filter(|dep| *dep != id); match stage { StageStart => { - resolve.deps(id).into_iter().flat_map(|a| a).filter(|dep| { - *dep != id + // Only transitive dependencies are needed to start building a + // package. Non transitive dependencies (dev dependencies) are + // only used to build tests. + deps.filter(|dep| { + let dep = pkg.get_dependencies().iter().find(|d| { + d.get_name() == dep.get_name() + }).unwrap(); + dep.is_transitive() }).map(|dep| { - (dep, StageEnd) + (dep, StageLibraries) }).collect() } StageCustomBuild => vec![(id, StageStart)], StageLibraries => vec![(id, StageCustomBuild)], StageBinaries => vec![(id, StageLibraries)], - StageEnd => vec![(id, StageBinaries), (id, StageLibraries)], + StageTests => { + let mut ret = vec![(id, StageLibraries)]; + ret.extend(deps.filter(|dep| { + let dep = pkg.get_dependencies().iter().find(|d| { + d.get_name() == dep.get_name() + }).unwrap(); + !dep.is_transitive() + }).map(|dep| { + (dep, StageLibraries) + })); + ret + } } } } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 0dc884f4f..2eda93ff8 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -9,8 +9,10 @@ use util::{CargoResult, ProcessBuilder, CargoError, human, caused_human}; use util::{Config, internal, ChainError, Fresh, profile}; use self::job::{Job, Work}; -use self::job_queue::{JobQueue, StageStart, StageCustomBuild, StageLibraries}; -use self::job_queue::{StageBinaries, StageEnd}; +use self::job_queue as jq; +use self::job_queue::JobQueue; +use self::context::{Context, PlatformRequirement, PlatformTarget}; +use self::context::{PlatformPlugin, PlatformPluginAndTarget}; pub use self::compilation::Compilation; pub use self::context::Context; @@ -68,7 +70,7 @@ pub fn compile_targets<'a>(env: &str, targets: &[&'a Target], pkg: &'a Package, let mut cx = try!(Context::new(env, resolve, sources, deps, config, host_layout, target_layout)); - let mut queue = JobQueue::new(cx.resolve, cx.config); + let mut queue = JobQueue::new(cx.resolve, deps, cx.config); // First ensure that the destination directory exists try!(cx.prepare(pkg)); @@ -133,7 +135,7 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package, let (plugin1, plugin2) = fingerprint::prepare_init(cx, pkg, KindPlugin); init.push((Job::new(plugin1, plugin2, String::new()), Fresh)); } - jobs.enqueue(pkg, StageStart, init); + jobs.enqueue(pkg, jq::StageStart, init); // First part of the build step of a target is to execute all of the custom // build commands. @@ -153,15 +155,15 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package, for cmd in build_cmds.into_iter() { try!(cmd()) } dirty() }; - jobs.enqueue(pkg, StageCustomBuild, vec![(job(dirty, fresh, desc), - freshness)]); + jobs.enqueue(pkg, jq::StageCustomBuild, vec![(job(dirty, fresh, desc), + freshness)]); // After the custom command has run, execute rustc for all targets of our // package. // // Each target has its own concept of freshness to ensure incremental // rebuilds on the *target* granularity, not the *package* granularity. - let (mut libs, mut bins) = (Vec::new(), Vec::new()); + let (mut libs, mut bins, mut tests) = (Vec::new(), Vec::new(), Vec::new()); for &target in targets.iter() { let work = if target.get_profile().is_doc() { let (rustdoc, desc) = try!(rustdoc(pkg, target, cx)); @@ -171,7 +173,11 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package, try!(rustc(pkg, target, cx, req)) }; - let dst = if target.is_lib() {&mut libs} else {&mut bins}; + let dst = match (target.is_lib(), target.get_profile().is_test()) { + (_, true) => &mut tests, + (true, _) => &mut libs, + (false, false) => &mut bins, + }; for (work, kind, desc) in work.into_iter() { let (freshness, dirty, fresh) = try!(fingerprint::prepare_target(cx, pkg, target, kind)); @@ -180,9 +186,9 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package, dst.push((job(dirty, fresh, desc), freshness)); } } - jobs.enqueue(pkg, StageLibraries, libs); - jobs.enqueue(pkg, StageBinaries, bins); - jobs.enqueue(pkg, StageEnd, Vec::new()); + jobs.enqueue(pkg, jq::StageLibraries, libs); + jobs.enqueue(pkg, jq::StageBinaries, bins); + jobs.enqueue(pkg, jq::StageTests, tests); Ok(()) } diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs index ad805d85e..6e022d634 100644 --- a/tests/test_cargo_compile_git_deps.rs +++ b/tests/test_cargo_compile_git_deps.rs @@ -1054,8 +1054,8 @@ test!(dev_deps_with_testing { // a second time. assert_that(p.process(cargo_dir().join("cargo")).arg("test"), execs().with_stdout(format!("\ -{compiling} bar v0.5.0 ({bar}#[..]) -{compiling} foo v0.5.0 ({url}) +{compiling} [..] v0.5.0 ([..]) +{compiling} [..] v0.5.0 ([..] {running} target[..]foo-[..] running 1 test @@ -1063,7 +1063,7 @@ test tests::foo ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured -", compiling = COMPILING, url = p.url(), running = RUNNING, bar = p2.url()))); +", compiling = COMPILING, running = RUNNING))); }) test!(git_build_cmd_freshness { diff --git a/tests/test_cargo_compile_path_deps.rs b/tests/test_cargo_compile_path_deps.rs index 1a4b611f8..583289cc6 100644 --- a/tests/test_cargo_compile_path_deps.rs +++ b/tests/test_cargo_compile_path_deps.rs @@ -175,8 +175,8 @@ test!(cargo_compile_with_root_dev_deps_with_testing { p2.build(); assert_that(p.cargo_process("test"), execs().with_stdout(format!("\ -{compiling} bar v0.5.0 ({url}) -{compiling} foo v0.5.0 ({url}) +{compiling} [..] v0.5.0 ({url}) +{compiling} [..] v0.5.0 ({url}) {running} target[..]foo-[..] running 0 tests @@ -697,3 +697,56 @@ test!(path_dep_build_cmd { execs().with_stdout("1\n")); }) +test!(dev_deps_no_rebuild_lib { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.5.0" + authors = [] + + [dev-dependencies.bar] + path = "bar" + + [lib] + name = "foo" + doctest = false + "#) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", r#" + [package] + + name = "bar" + version = "0.5.0" + authors = ["wycats@example.com"] + "#) + .file("bar/src/lib.rs", "pub fn bar() {}"); + p.build(); + assert_that(p.process(cargo_dir().join("cargo")).arg("build"), + execs().with_status(0) + .with_stdout(format!("{} foo v0.5.0 ({})\n", + COMPILING, p.url()))); + p.root().move_into_the_past().assert(); + + // Now that we've built the library, it *should not* be built again as part + // of `cargo test`, even if we have some dev dependencies that weren't + // previously built. + File::create(&p.root().join("src/lib.rs")).write_str(r#" + #[cfg(test)] extern crate bar; + #[cfg(not(test))] fn foo() { bar(); } + "#).unwrap(); + p.root().join("src/lib.rs").move_into_the_past().assert(); + + assert_that(p.process(cargo_dir().join("cargo")).arg("test"), + execs().with_status(0) + .with_stdout(format!("\ +{} [..] v0.5.0 ({}) +{} [..] v0.5.0 ({}) +Running target[..]foo-[..] + +running 0 tests + +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured + +", COMPILING, p.url(), COMPILING, p.url()))); +}) -- 2.30.2